feat(java): raise baseline to 17 and add workflow gates#331
Conversation
Java 11→17 so Spring Boot 3 apps can adopt and to enable records/ sealed types. CI JDK matrix → 17/21/25.
Park a step (WAITING_APPROVAL) until Worker.approveGate/rejectGate or its timeout resolves it; successors then run. Adds gate/node JNI fns + workflowNameForRun, GateConfig/GateAction, Step.gate/Workflow.gate, and a tracker that promotes deferred nodes from registered workflows.
Approve, reject, and timeout (auto-approve/auto-reject) paths.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds workflow gate nodes and metadata end-to-end, threads parent run linkage and gate state JNI methods through Java and Rust, implements gate promotion and timeout handling in ChangesWorkflow Gate Feature and Java 17 Upgrade
Sequence Diagram(s)sequenceDiagram
participant DefaultTaskito
participant JniQueueBackend
participant NativeWorkflows
participant WorkflowTracker
participant RustJNI
DefaultTaskito->>JniQueueBackend: submitWorkflow(...)
JniQueueBackend->>NativeWorkflows: submitWorkflow(handle, ..., parentRunId, parentNodeName)
NativeWorkflows->>RustJNI: Java_org_byteveda_taskito_internal_NativeWorkflows_submitWorkflow
WorkflowTracker->>JniQueueBackend: resolveWorkflowGate(runId, nodeName, approved, error)
JniQueueBackend->>NativeWorkflows: resolveWorkflowGate(handle, runId, nodeName, approved, error)
NativeWorkflows->>RustJNI: Java_org_byteveda_taskito_internal_NativeWorkflows_resolveWorkflowGate
Estimated code review effort🎯 5 (Critical) | ⏱️ ~100 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PMD (7.25.0)sdks/java/src/main/java/org/byteveda/taskito/workflows/GateConfig.java[ERROR] Cannot load ruleset .coderabbit-pmd-ruleset.xml: Cannot resolve rule/ruleset reference '.coderabbit-pmd-ruleset.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. sdks/java/src/main/java/org/byteveda/taskito/workflows/Step.java[ERROR] Cannot load ruleset .coderabbit-pmd-ruleset.xml: Cannot resolve rule/ruleset reference '.coderabbit-pmd-ruleset.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. sdks/java/src/main/java/org/byteveda/taskito/workflows/WorkflowTracker.java[ERROR] Cannot load ruleset .coderabbit-pmd-ruleset.xml: Cannot resolve rule/ruleset reference '.coderabbit-pmd-ruleset.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/taskito-java/src/workflows/mod.rs (1)
571-580: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve the run’s queue default in plan metadata.
getWorkflowPlanstill serializes only the per-step queue. Deferred jobs created later byWorkflowTrackercannot recover thequeue_defaultthat was used for static jobs at submit time, so deferred successors can be enqueued on the wrong default queue. Persist or expose the effective queue for deferred nodes instead of relying onPlanNode.queueOrDefault().Based on learnings,
getWorkflowPlanmay omit the run default queue, which later breaks deferred job creation inWorkflowTracker; explicit.queue(...)is only the current workaround and the long-term fix is native serialization of the run default queue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/taskito-java/src/workflows/mod.rs` around lines 571 - 580, The workflow plan serialization currently drops the run-level queue default, so deferred jobs later built by WorkflowTracker can’t recover the effective queue and may use the wrong one. Update getWorkflowPlan to persist or expose the effective queue for deferred nodes, using PlanNode.queueOrDefault() or an equivalent resolved queue field instead of only serializing the per-step queue. Make sure the plan metadata includes enough information for WorkflowTracker to enqueue deferred successors with the same default queue that was active at submit time.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/taskito-java/src/workflows/mod.rs`:
- Around line 230-232: The workflow reuse path in mod.rs is only checking same
DAG name/version, but `StepMetadata` fields like `condition`, `gate`, and
`sub_workflow` can change execution behavior and must be part of compatibility.
Update the existing-definition lookup in the workflow step handling code to
compare `step_meta` as well, or reject reuse and require a version bump when the
metadata differs. Use the `StepMetadata`/step reuse logic around the
`condition`, `gate`, and `sub_workflow` assignment to locate the compatibility
check and ensure stale metadata cannot be reused.
- Around line 588-610: `workflowNameForRun` currently returns only `def.name`,
which can collide across workflow versions and make deferred payload resolution
pick the wrong workflow. Update
`Java_org_byteveda_taskito_internal_NativeWorkflows_workflowNameForRun` and the
related tracker path to expose a versioned workflow identity, or add a separate
version accessor, and ensure the Java tracker caches deferred payloads using
both workflow name and version. Use the existing `wf.get_workflow_run(...)` and
`get_workflow_definition_by_id(...)` lookup flow to retrieve the version
alongside the name and return/store both fields consistently.
- Around line 1050-1055: The node is still being marked Skipped in the workflow
update flow even when queue.storage.cancel_job fails, which can leave a running
bound job behind. In the workflow handling around wf.get_workflow_node, capture
and propagate the cancel_job result instead of discarding it, or only suppress
explicitly safe cases like already terminal/not found, and only call
wf.update_workflow_node_status to Skipped after cancellation has succeeded.
In `@sdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.java`:
- Around line 459-460: The workflow definition reuse logic is too permissive:
`DefaultTaskito` only compares DAG topology, so changes to behavior-bearing gate
metadata can be ignored and stale `StepMetadata.gate` may be reused. Update the
definition compatibility check in the native submit path to include step
metadata (especially gate fields encoded via `encodeGate`), or explicitly reject
resubmits under the same version when metadata drifts. Make sure the stored
definition used later by the tracker cannot silently carry over old gate
timeout/action/message settings.
In `@sdks/java/src/main/java/org/byteveda/taskito/workflows/GateConfig.java`:
- Around line 30-37: Reject null in the GateConfig.timeout factory methods: both
timeout(Duration, GateAction) and timeout(Duration, GateAction, String) should
validate that the timeout argument is non-null instead of passing it through to
the record constructor, since GateConfig currently treats a null timeout as an
indefinite manual gate. Add an explicit null check or equivalent validation in
these timeout overloads so misconfigured auto-resolution paths fail fast rather
than creating a wait-forever gate.
In `@sdks/java/src/main/java/org/byteveda/taskito/workflows/Step.java`:
- Around line 120-136: The public Step.Builder.gate(...) setter is allowing
normal task steps to be marked as gates, but the workflow execution path treats
any step with gate != null as a deferred control node and skips enqueuing its
task payload. Update Step.Builder.build() and related gate validation so gates
are only allowed on Workflow.gate(...) / __gate__ control nodes, or otherwise
change the execution/tracking flow to enqueue the task after approval; use the
Step.Builder.gate(...) and Step.build() symbols to locate the fix.
In `@sdks/java/src/main/java/org/byteveda/taskito/workflows/Workflow.java`:
- Around line 82-90: The gate path in Workflow.gate currently leaves downstream
deferred nodes vulnerable to the getWorkflowPlan queue-default gap, so
WorkflowTracker can recreate them on "default" instead of the intended lane.
Update the gate/deferred-node handling in Workflow.gate and the related workflow
plan generation so the run default queue is propagated for deferred successors,
or enforce/fail fast when a deferred step lacks an explicit .queue(...) and
document that requirement clearly in the gate API behavior.
In `@sdks/java/src/main/java/org/byteveda/taskito/workflows/WorkflowTracker.java`:
- Around line 253-268: In WorkflowTracker.resolveGate, only consume the
idempotency entry for gates that are actually parked by checking the node
snapshot state is WAITING_APPROVAL before adding to resolvedGates. Keep the
resolvedGates key until after backend.resolveWorkflowGate(runId, nodeName,
approved, error) succeeds, and if resolution fails, do not leave the gate marked
resolved. Use the existing resolveGate, resolvedGates, gateTimers, and
backend.resolveWorkflowGate symbols to update the flow so early
approveGate/rejectGate calls on PENDING nodes do not promote successors
prematurely.
- Around line 213-225: In WorkflowTracker.promoteOutcome, the promotedNodes
dedupe is applied before the promotion side effects, so if enterGate(...) or
createDeferredJobFor(...) throws the node remains half-promoted and future
retries will be blocked. Update the logic around the GateKey add check to roll
back the dedupe entry when promotion fails, or explicitly mark the node failed
before returning, so subsequent outcome handling can retry or fail cleanly.
Focus on the promoteOutcome flow and the enterGate/createDeferredJobFor paths.
---
Outside diff comments:
In `@crates/taskito-java/src/workflows/mod.rs`:
- Around line 571-580: The workflow plan serialization currently drops the
run-level queue default, so deferred jobs later built by WorkflowTracker can’t
recover the effective queue and may use the wrong one. Update getWorkflowPlan to
persist or expose the effective queue for deferred nodes, using
PlanNode.queueOrDefault() or an equivalent resolved queue field instead of only
serializing the per-step queue. Make sure the plan metadata includes enough
information for WorkflowTracker to enqueue deferred successors with the same
default queue that was active at submit time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4266d8c-68c9-4cf3-9786-352ac534150e
📒 Files selected for processing (19)
.github/actions/setup-java/action.yml.github/workflows/ci-java.ymlcrates/taskito-java/src/workflows/mod.rssdks/java/build.gradle.ktssdks/java/processor/build.gradle.ktssdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.javasdks/java/src/main/java/org/byteveda/taskito/internal/JniQueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/internal/NativeWorkflows.javasdks/java/src/main/java/org/byteveda/taskito/spi/QueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/worker/Worker.javasdks/java/src/main/java/org/byteveda/taskito/workflows/GateAction.javasdks/java/src/main/java/org/byteveda/taskito/workflows/GateConfig.javasdks/java/src/main/java/org/byteveda/taskito/workflows/PlanNode.javasdks/java/src/main/java/org/byteveda/taskito/workflows/Step.javasdks/java/src/main/java/org/byteveda/taskito/workflows/Workflow.javasdks/java/src/main/java/org/byteveda/taskito/workflows/WorkflowTracker.javasdks/java/src/test/java/org/byteveda/taskito/WorkflowGateTest.javasdks/java/test-support/build.gradle.ktssdks/java/test-support/src/main/java/org/byteveda/taskito/test/InMemoryQueueBackend.java
Gradle 8.12 cannot run on JDK 25; bump wrapper to 9.6.1, spotless 7.2.1, vanniktech 0.37.0. Migrate registering->register and drop the removed SonatypeHost arg for Gradle 9 / plugin compat.
palantir-java-format uses javac internals that JDK 25 changed; keep spotless/checkstyle on the baseline JDK and run the test suite on every JDK.
Changes
build.gradle.ktscompiles with--release 17(no pinned toolchain; modern JDK compiles,--releaseblocks newer-API leak). Unlocks records, sealed interfaces, pattern-matchingswitch, and Spring Boot 3. Pre-1.0 floor bump from 11→17.WaitingApproval;Queue.approveGate/rejectGateresolve them (approve runs successors, reject skip-cascades). New JNI fns (setWorkflowNodeWaitingApproval,resolveWorkflowGate, …) over the existingtaskito-workflowsstorage; tracker drives gate state. No new core storage.Tests
Workflow-gate coverage added.
./gradlew buildgreen (Spotless + Checkstyle + tests); native.sobundled.Notes
Bottom of a stacked series — subsequent PRs (conditions, sub-workflows, sagas, resources, predicates, codecs, FFM fast-path, mesh, …) branch off this one in order.
Summary by CodeRabbit
New Features
Bug Fixes
Chores